feat: install Python and Node packages in container #1220
Conversation
|
Now tested locally with Singularity and the Devcontainer, and both are working. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1220 +/- ##
=======================================
Coverage 46.05% 46.05%
=======================================
Files 11 11
Lines 4942 4942
Branches 1345 1345
=======================================
Hits 2276 2276
Misses 2666 2666
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I'm seeing "RuntimeError". Using podman. You are apparently not seeing that, so I'm wondering what the differences are. With this history, I marked the commits that worked (OK) and didn't (KO):
KO: 6d546f8b571a (jordancarlin/install_deps, install_deps) update more node_module paths
6da03ef3df1d Fix context in devcontainer
1ec2d03e3ceb Move requirements.txt install location to work around Singularity clearing /tmp
87c62f446142 update package-lock.json
c03df7b9290d install npm packages in container
KO: 977232813d49 (HEAD) Install Python packages in container
OK: e3fb9b1f6178 feat: attempt to pull docker image before building it (#1218)
7e6266dafd64 feat(data): add Zihintntl instructions (#1213)
OK: ea744dd24cc7 fix(typo): correct typo in Xqci extension description (#1214)
Hmm. I built the image and ran regression successfully with Podman, Singularity, and a Devcontainer. Can you include the actual error message and what exact command triggered it to help debug? |
|
|
BTW, I usually run a shell in a container, and that has additional things that are not available outside of the container, like "bundle": ...maybe that's causing the issue here? If so, I wonder if the "fix" is that "bundle" needs to be installed somewhere in the setup scripts? (Do I need to install "bundle" outside of the container? That seems counter-intuitive to using the containers.) |
|
Looking at the log you shared, I'm guessing it didn't actually rebuild the container. So you are running the new scripts that expect binaries in different places, but your container still has them in the old places. I'll look into why that might be happening, but in the meantime you could probably work around it by removing the old image ( |
|
I didn't bump the container version that it was looking for, so I think it pulled the existing v0.9 image from Docker Hub. Just pushed another commit that increases the container version to 0.10, so hopefully it will now detect that there is no compatible image on Docker Hub and will build the image itself locally. Once we merge this, it looks like there is a CI job that will publish the new version to Docker Hub. @ThinkOpenly can you test again with the latest version? |
It worked with that change. 🎉 |
It looks like the last couple runs of the image publish job failed for unrelated reasons: https://github.com/riscv-software-src/riscv-unified-db/actions/workflows/container.yml. We need to get that resolved before merging this, otherwise the new image won't be available to users. |
The failures appear to have been intermittent. It failed trying to download/install packages, so perhaps a network glitch. I restarted the first failure, and it completed successfully. I have restarted the 2nd, but I expect the same. |
| ENV PATH="/opt/venv/bin:$PATH" | ||
|
|
||
| # Install npm packages globally in the container | ||
| COPY package.json package-lock.json /opt/node/ |
There was a problem hiding this comment.
with the npm install happening in the container now, shouldn't we drop package-lock.json from the repo entirely? It's not going to be kept up to date since npm no longer runs at the root
There was a problem hiding this comment.
I'm installing packages using npm ci right now, which uses the versions from package-lock.json. We were previously using npm i which didn't actually fix the versions. I'd be in favor of marinating the lock file so that all of the dependencies are clear.
It might make sense to enable Dependabot for npm packages so that we get automatically get PRs to bump them.
3 participants
Python, Node, and Ruby packages were all installed in the local workspace at runtime. This slows down initial runs and creates the potential for people to use different versions of packages (the Node lock file was not actually being used). It also creates permissions issues when switching between different container runtimes (I've been switching between Podman and the Devcontainer lately).
This installs the Python and Node packages in the container and updates the
npminstallation process to actually usepackage-lock.json.I haven't tested with Singularity(I'm a Podman/Docker user), but everything is passing with Podman locally.I'll follow up with another PR later to migrate some of the Ruby packages to the container as well, but that one is trickier because some of the Ruby packages are local to the UDB project.